Closed
Conversation
📝 WalkthroughWalkthroughThe changes rename references from "result service" to "storage service" across nginx configuration generation and cleanup utilities, including variable names, proxy targets, comments, and pod lookup logic. No public APIs or function signatures are modified. Changes
Estimated code review effort🎯 1 (Trivial) | ⏱️ ~5 minutes Possibly related issues
Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing touches
🧪 Generate unit tests (beta)
Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
src/k8s/kubernetes.py (1)
387-441:⚠️ Potential issue | 🟠 MajorGuard against non-unique or missing storage-service lookup.
get_k8s_resource_namesmay returnNoneorlist[str], which would render an invalidproxy_passtarget. Consider enforcing a single service name before generating the config.🔧 Proposed fix
storage_service_name = get_k8s_resource_names('service', 'label', 'component=flame-storage-service', namespace=namespace) + if not isinstance(storage_service_name, str): + raise ValueError( + f"Expected a single storage-service name, got: {storage_service_name!r}" + )
🤖 Fix all issues with AI agents
In `@src/resources/utils.py`:
- Around line 215-222: get_k8s_resource_names may return None or a list; before
calling delete_resource you must normalize storage_service_name to a list, skip
when empty/None, and call delete_resource for each pod name. Specifically, in
the block that handles cleanup_type (where storage_service_name is assigned from
get_k8s_resource_names), replace the direct delete_resource call with logic
that: (1) checks if storage_service_name is truthy, (2) if it's a string convert
it into a single-item list, (3) iterate the list and call delete_resource(name,
'pod', namespace) for each entry, and (4) set response_content[cleanup_type]
only after performing deletions (or set a helpful message if no pods were
found). Ensure you reference the existing symbols get_k8s_resource_names,
delete_resource, storage_service_name, cleanup_type, namespace, and
response_content.
Comment on lines
215
to
+222
| if cleanup_type in ['all', 'services', 'rs']: | ||
| # reinitialize result-service pod | ||
| result_service_name = get_k8s_resource_names('pod', | ||
| # reinitialize storage-service pod | ||
| storage_service_name = get_k8s_resource_names('pod', | ||
| 'label', | ||
| 'component=flame-result-service', | ||
| 'component=flame-storage-service', | ||
| namespace=namespace) | ||
| delete_resource(result_service_name, 'pod', namespace) | ||
| response_content[cleanup_type] = "Reset result service" | ||
| delete_resource(storage_service_name, 'pod', namespace) | ||
| response_content[cleanup_type] = "Reset storage service" |
There was a problem hiding this comment.
Handle multiple or missing storage-service pods during cleanup.
get_k8s_resource_names('pod', ...) can return None or list[str]. Passing either to delete_resource will fail. Consider normalizing to a list and skipping when empty.
🔧 Proposed fix
- storage_service_name = get_k8s_resource_names('pod',
- 'label',
- 'component=flame-storage-service',
- namespace=namespace)
- delete_resource(storage_service_name, 'pod', namespace)
- response_content[cleanup_type] = "Reset storage service"
+ storage_service_names = get_k8s_resource_names('pod',
+ 'label',
+ 'component=flame-storage-service',
+ namespace=namespace)
+ if storage_service_names is None:
+ response_content[cleanup_type] = "No storage service pod found"
+ else:
+ if isinstance(storage_service_names, str):
+ storage_service_names = [storage_service_names]
+ for pod_name in storage_service_names:
+ delete_resource(pod_name, 'pod', namespace)
+ response_content[cleanup_type] = "Reset storage service"🤖 Prompt for AI Agents
In `@src/resources/utils.py` around lines 215 - 222, get_k8s_resource_names may
return None or a list; before calling delete_resource you must normalize
storage_service_name to a list, skip when empty/None, and call delete_resource
for each pod name. Specifically, in the block that handles cleanup_type (where
storage_service_name is assigned from get_k8s_resource_names), replace the
direct delete_resource call with logic that: (1) checks if storage_service_name
is truthy, (2) if it's a string convert it into a single-item list, (3) iterate
the list and call delete_resource(name, 'pod', namespace) for each entry, and
(4) set response_content[cleanup_type] only after performing deletions (or set a
helpful message if no pods were found). Ensure you reference the existing
symbols get_k8s_resource_names, delete_resource, storage_service_name,
cleanup_type, namespace, and response_content.
Member
|
Changes have been applied in #41 (no clue why this branch was left loose) |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary by CodeRabbit